Skip to content

🛡️ Sentinel: [HIGH] Fix Zip Slip vulnerability in batch ZIP generation - #345

Closed
seonghobae wants to merge 3 commits into
mainfrom
sentinel/fix-zip-slip-14064353898408349512
Closed

🛡️ Sentinel: [HIGH] Fix Zip Slip vulnerability in batch ZIP generation#345
seonghobae wants to merge 3 commits into
mainfrom
sentinel/fix-zip-slip-14064353898408349512

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

🚨 Severity: HIGH
💡 Vulnerability: 사용자가 백슬래시(\)를 포함한 조작된 파일 이름(예: ..\..\etc\passwd)으로 파일을 업로드할 때, POSIX 시스템의 Path.name이 백슬래시를 경로 구분자로 인식하지 않아 그대로 유지되며, 그 결과 다운로드되는 ZIP 아카이브에 악의적인 경로(Zip Slip)가 포함됩니다.
🎯 Impact: 사용자가 다운로드된 ZIP 파일을 Windows 시스템에서 추출할 때 의도하지 않은 디렉토리 경로(예: 시스템 폴더)에 파일이 덮어써져 보안 피해를 입을 수 있습니다.
🔧 Fix: 파일 이름을 추출하기 전에 백슬래시(\)를 슬래시(/)로 변환하여 Path().name이 올바르게 파일의 기저 이름(basename)만을 추출하도록 수정했습니다.
✅ Verification: tests/test_saas_web.py 내의 단위 테스트(test_shrink_batch_sanitizes_windows_path_traversal)를 통해 검증되었습니다.


PR created automatically by Jules for task 14064353898408349512 started by @seonghobae

@google-labs-jules

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d07495b4-f271-499e-81ba-6089fc7dc0e9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@opencode-agent @cwl-noema-review

Rebuild this Draft from exact protected current main. Remove .github/workflows/repair-pr-345.yml, encoded patches, and every one-shot branch writer. Implement one centralized, documented filename-boundary function used by single upload, batch upload, async upload, result naming, and ZIP member creation. Treat both / and \\ as separators regardless of runner OS; reject or deterministically replace empty, ., .., NUL/control characters, absolute/drive/UNC syntax, trailing Windows dot/space ambiguity, reserved device names, and unsafe normalization results; preserve a safe extension where possible; prevent case-insensitive and normalization-equivalent collisions inside a batch by deterministic unique naming; never expose original hostile paths in errors. Add direct property/fuzz tests and end-to-end ZIP tests that inspect every member as both POSIX and Windows paths, then extract with representative standard libraries into a bounded temp root and prove no escape/overwrite. Preserve current dynamic upload limits, total batch-size validation, cleanup behavior, accessibility, and all accepted security fixes. Add CWE-22/ZIP archive/Windows path doctoring with APA 7 primary references, update the current Unreleased changelog, and run hash-pinned install, focused/complete tests, 100% production statement/branch coverage, production docstrings, compilation, fuzz/property tests, package smoke, security/SAST, and git diff --check. Publish a normal source commit only. Do not merge, approve, release, alter protections, or retain duplicate filename sanitizers.

Copy link
Copy Markdown
Contributor Author

Superseded by #371. This old-base Draft handles only one backslash traversal example and does not define a complete cross-platform filename or ZIP-member boundary. The replacement issue covers all upload/result/archive call sites, Windows drive/UNC/device and trailing-dot semantics, Unicode/case collisions, deterministic duplicate naming, bounded extraction tests, 100% coverage/docstrings, and CSAP/SOC 2 evidence. Closing without merge avoids treating a partial security patch as complete remediation.

@seonghobae seonghobae closed this Aug 7, 2026
@google-labs-jules

Copy link
Copy Markdown

@opencode-agent @cwl-noema-review

Rebuild this Draft from exact protected current main. Remove .github/workflows/repair-pr-345.yml, encoded patches, and every one-shot branch writer. Implement one centralized, documented filename-boundary function used by single upload, batch upload, async upload, result naming, and ZIP member creation. Treat both / and \\ as separators regardless of runner OS; reject or deterministically replace empty, ., .., NUL/control characters, absolute/drive/UNC syntax, trailing Windows dot/space ambiguity, reserved device names, and unsafe normalization results; preserve a safe extension where possible; prevent case-insensitive and normalization-equivalent collisions inside a batch by deterministic unique naming; never expose original hostile paths in errors. Add direct property/fuzz tests and end-to-end ZIP tests that inspect every member as both POSIX and Windows paths, then extract with representative standard libraries into a bounded temp root and prove no escape/overwrite. Preserve current dynamic upload limits, total batch-size validation, cleanup behavior, accessibility, and all accepted security fixes. Add CWE-22/ZIP archive/Windows path doctoring with APA 7 primary references, update the current Unreleased changelog, and run hash-pinned install, focused/complete tests, 100% production statement/branch coverage, production docstrings, compilation, fuzz/property tests, package smoke, security/SAST, and git diff --check. Publish a normal source commit only. Do not merge, approve, release, alter protections, or retain duplicate filename sanitizers.

Understood. The request has been superseded and this work is now obsolete. Stopping work on this task.

@google-labs-jules

Copy link
Copy Markdown

Superseded by #371. This old-base Draft handles only one backslash traversal example and does not define a complete cross-platform filename or ZIP-member boundary. The replacement issue covers all upload/result/archive call sites, Windows drive/UNC/device and trailing-dot semantics, Unicode/case collisions, deterministic duplicate naming, bounded extraction tests, 100% coverage/docstrings, and CSAP/SOC 2 evidence. Closing without merge avoids treating a partial security patch as complete remediation.

Acknowledged. Closing this out as obsolete since it has been superseded by a complete cross-platform fix in #371.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant